Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 3, 2025

Addresses review feedback requesting complete API documentation for get_exchange_kernels_GaussLegendre.

Changes

  • Added NumPy-style docstring with:
    • Parameters: All 9 arguments documented with types and semantics
    • Returns: Explicit numpy.ndarray of numpy.complex128 with shape specification
    • Raises: Specific error conditions including callable shape requirements
    • Notes: Preserved optimization details in proper section format
def get_exchange_kernels_GaussLegendre(
    G_magnitudes,
    G_angles,
    nmax: int,
    *,
    potential: str | callable = "coulomb",
    kappa: float = 1.0,
    nquad: int = 8000,
    scale: float = 0.5,
    ell: float = 1.0,
    sign_magneticfield: int = -1,
) -> "ComplexArray":
    """Compute exchange kernels X_{n1,m1,n2,m2}(G) using Gauss-Legendre quadrature.

    Parameters
    ----------
    G_magnitudes : array_like of float
        Magnitudes |G| of the reciprocal lattice vectors...
    ...

    Returns
    -------
    numpy.ndarray of numpy.complex128
        Exchange kernels with shape ``(nG, nmax, nmax, nmax, nmax)``...
    """

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI self-assigned this Dec 3, 2025
Copilot AI and others added 2 commits December 3, 2025 19:23
Co-authored-by: skilledwolf <18141588+skilledwolf@users.noreply.github.com>
Co-authored-by: skilledwolf <18141588+skilledwolf@users.noreply.github.com>
Copilot AI changed the title [WIP] Enhance docstring based on review feedback for indexing fixes Add comprehensive docstring for get_exchange_kernels_GaussLegendre Dec 3, 2025
Copilot AI requested a review from skilledwolf December 3, 2025 19:27
@skilledwolf skilledwolf marked this pull request as ready for review December 3, 2025 19:37
Copilot AI review requested due to automatic review settings December 3, 2025 19:37
@skilledwolf skilledwolf merged commit 5f80dea into Fixes_B_field Dec 3, 2025
@skilledwolf skilledwolf deleted the copilot/sub-pr-2-another-one branch December 3, 2025 19:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds comprehensive NumPy-style documentation to the get_exchange_kernels_GaussLegendre function, addressing a review feedback request for complete API documentation. The docstring follows the codebase's established NumPy documentation style with complete parameter descriptions, return value specification, error conditions, and implementation notes.

Key changes:

  • Expanded one-line summary to a detailed multi-paragraph description of the function's purpose
  • Added complete parameter documentation for all 9 arguments including types, semantics, and default values
  • Specified explicit return type (numpy.ndarray of numpy.complex128) with shape information
  • Documented three distinct error conditions in the Raises section
  • Restructured optimization details into proper Notes section with improved formatting
Comments suppressed due to low confidence (1)

src/quantumhall_matrixelements/exchange_legendre.py:160

  • Variable Veff is not used.
        Veff = None

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +87 to +91
sign_magneticfield : int, optional
Sign of the charge–field product σ = sgn(q B_z). Must be -1 or +1.
Default is -1 (internal convention). Use +1 to obtain kernels for
the opposite magnetic field orientation via conjugation and phase
factors.
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring states that sign_magneticfield "Must be -1 or +1", but the function does not validate this constraint. The related function get_exchange_kernels_hankel includes explicit validation (if sign_magneticfield not in (1, -1): raise ValueError(...)). Consider either:

  1. Adding validation at the start of the function to enforce this constraint, or
  2. Softening the language to "Should be -1 or +1" or "Typically -1 or +1"

This ensures consistency between documentation and actual behavior.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants